Skip to content

Give Python callers the findings the CLI already prints (#104) - #481

Open
jeremymanning wants to merge 2 commits into
mainfrom
fix/validation-api
Open

Give Python callers the findings the CLI already prints (#104)#481
jeremymanning wants to merge 2 commits into
mainfrom
fix/validation-api

Conversation

@jeremymanning

Copy link
Copy Markdown
Member

Review item 4. Merge after #480 — this branch is stacked on it, so its diff includes #480's commit until that lands.

The gap

orchestrator validate --json has emitted structured findings since #467. PipelineAPI.validate_yaml returns a bare bool, so a caller embedding the orchestrator could learn that a document was rejected and nothing about why — and could not see warnings at all, which is where "this reference could not be checked" lives: the warning that precedes the run-time failure in #465.

The findings were never missing, only private to cli.py (_reportable_issues, _issue_payload).

Why this is a move, not an addition

Writing a second implementation for the API would have created two things to drift apart — the bug #466 removed for dependencies. validation/pipeline_report.py is the one implementation; the CLI now formats what it returns, and its private helpers are deleted. test_the_cli_and_the_api_report_the_same_findings compares the CLI's JSON against the API's objects to keep it that way.

Exported from the package: validate_pipeline_file, validate_pipeline_text, PipelineValidation, Finding.

Compilation failure is a result, not an exception: a caller validating user input wants the findings alongside the failure, and raising discards them at the moment they are most useful.

orchestrator validate was not reproducible

Your list asked for deterministic warning order. It wasn't:

5f5b24bbc1e0
31dae9338721   ← same command, same file, three runs
b882273628de

Same 44 findings, three orders. Two hash-order sources:

  1. findings emitted while iterating set(var_names) | set(loop_var_matches);
  2. _suggest_similar_names iterating an unsorted candidate list and truncating to three — so on a longer list it would have offered genuinely different suggestions run to run, not merely reordered ones.

Both sorted. Three separate processes now produce byte-identical output.

Two of my own tests proved nothing

Reporting these because they are the reason to trust the rest:

  • The suite was first written against examples/supported/01_hello_filesystem.yaml, which produces no findings. Deleting every warning from the implementation still passed all seven tests — a suite for "warnings reach the API" that could not detect warnings never arriving. It is now anchored on a document carrying 44 findings, with explicit assert result.findings guards so an empty list cannot pass as agreement.
  • The determinism test ran in one process, where PYTHONHASHSEED is fixed, and passed while the CLI visibly varied. The real check runs three subprocesses.

Neither was caught by rereading the tests; both were caught by mutating the implementation and checking that a test failed.

Verification

  • ruff check src/orchestrator --select E9,F63,F7,F82,F821,F823,F601,F811 — clean
  • Full blocking suite — 1027 passed, 13 skipped
  • Catalogue — all 52 listed examples still validate
  • Determinism — 3 separate processes, identical digests
  • Three mutations, each caught: warnings dropped from findings; unsorted variable iteration; unsorted suggestion candidates

Gates were re-run on the rebased branch in an isolated git worktree, after an earlier post-rebase run was invalidated by the shared checkout switching branches mid-run.

jeremymanning and others added 2 commits August 4, 2026 12:10
#471 established which parts of a step the runtime renders and wired that into
dependency inference, so two inert strings stopped inventing a cycle. Template
validation was still reading those same fields as if they resolved, and got
both directions wrong:

    - id: a
      name: "{{ b.result }}"          # "will be resolved at runtime"
      description: "{{ nosuch }}"     # a hard error

Nothing substitutes into `name`, so the braces reach the log verbatim and the
warning told the reader the opposite of what happens. And a stray brace in
prose *failed the pipeline* -- a false rejection of a document that runs
correctly, which is the class removed by #465, #469 and #472 elsewhere.

Inert fields now produce a warning that says what actually occurs, and nothing
else. Renderable fields are untouched: an undefined name in `parameters`,
`action` or `location` is still an error.

The same defect exists one level up -- a pipeline's own `name:` was rejected
too -- so `INERT_PIPELINE_FIELDS` sits alongside the step set. It is
deliberately shorter than it first was: `version` is schema-constrained to
`\d+\.\d+\.\d+`, so a template there is a real error and calling the field
inert would describe it wrongly. The run in
`test_a_pipeline_with_templates_in_prose_still_runs` is what caught that, and
is the evidence for every field in both sets: a pipeline carrying an
unresolvable reference in each prose field still executes and still writes the
right contents.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`orchestrator validate --json` has emitted structured findings since #467.
`PipelineAPI.validate_yaml` returns a bare `bool`, so a caller embedding the
orchestrator could learn *that* a document was rejected and nothing about why,
and could not see warnings at all -- which is where "this reference could not
be checked" lives, the warning that precedes the run-time failure in #465.

The findings were not missing, only private to `cli.py`. Exposing them by
writing a second implementation would have created two things to drift apart,
which is the bug #466 removed for dependencies, so `validation/pipeline_report`
is the one implementation and the CLI formats what it returns. A test compares
the CLI's JSON against the API's objects to keep it that way.

While testing it, `validate` turned out not to be reproducible. Three runs of
the same command over the same file produced the same 44 findings in three
different orders, from two hash-order sources:

* findings were emitted while iterating `set(var_names) | set(...)`;
* `_suggest_similar_names` iterated an unsorted candidate list *and truncates
  to three*, so on a longer list it would have offered different suggestions
  run to run rather than merely reordering them.

Both are now sorted, and repeated runs are byte-identical. The in-process
check cannot catch this -- `PYTHONHASHSEED` is fixed for the life of a
process -- so the test runs three subprocesses.

The first version of this suite was written against
`examples/supported/01_hello_filesystem.yaml`, which produces no findings at
all: deleting every warning from the implementation still passed all of it. It
is now anchored on a document carrying 44 findings, with explicit guards so an
empty list cannot pass as agreement.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant